Skip to content

Conversation

@okayvlev
Copy link

No description provided.

@okayvlev
Copy link
Author

Новое сообщение добавляется нажатием 'n', старые сообщения удаляются на 'd'.

@underoot underoot self-assigned this Mar 25, 2019
js/app.js Outdated
}

var currentMessage = messages[index];
currentMessage.setAttribute("state", "deleted");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кастомные аттрибуты у узлов лучше сторить в неймспейсе data-*

js/app.js Outdated

var currentMessage = messages[index];
currentMessage.setAttribute("state", "deleted");
var parent = currentMessage.parentNode;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для чего данная переменная?

border-width: 0px;
}

@keyframes appearance {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно анимацию для добавления письма организовать через transition

js/app.js Outdated

function newMessage() {
var currentMessage = document.getElementsByClassName("mailbox__mail")[0];
if (currentMessage.getAttribute("state") != "hidden") {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хорошо бы проработать кейс добавления не одного письма а нескольких

@@ -0,0 +1,36 @@
document.onkeypress = function(event) {
if (event.key == 'n') {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кроме клавиатурных сокращений попробуй организовать удаление писем по выделенным при нажатии на кнопку-ссылку Удалить

js/app.js Outdated
return;
}
currentMessage.setAttribute("state", "showing");
setTimeout(function() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно повесить обработчик на событие окончания анимации animationend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants